Feature: #8134 - copy to clipboard #8136
Conversation
pascalwengerter
force-pushed
the
feature/8134
branch
2 times, most recently
from
56904df
to
7dbe88c
Compare
|
Results for acceptance oC10 https://drone.owncloud.com/owncloud/web/30865/36/1 💥 The acceptance tests failed on retry. Please find the screenshots inside ...
webUISharingPublicBasic-publicLinkCreate_feature-L165.png |
packages/web-app-files/tests/unit/components/SideBar/PrivateLinkItem.spec.ts
Outdated
Show resolved
Hide resolved
|
I tried running the acceptance tests according to the guide in the docs, but it wouldn't work on my M1 Mac (failed with "outdated browser") Failing in https://drone.owncloud.com/owncloud/web/30836/36/19 in CI |
packages/web-app-files/tests/unit/components/SideBar/PrivateLinkItem.spec.ts
Outdated
Show resolved
Hide resolved
pascalwengerter
force-pushed
the
feature/8134
branch
from
1b24555
to
59da528
Compare
|
Results for acceptance oCIS https://drone.owncloud.com/owncloud/web/30887/53/1 |
pascalwengerter
force-pushed
the
feature/8134
branch
from
59da528
to
fa8e18c
Compare
I will, as pointed out above I couldn't get them to run locally as it worked in the past? I'll re-check one of these days, maybe I messed up some docker thing with the M1 chip |
pascalwengerter
force-pushed
the
feature/8134
branch
from
fa8e18c
to
1ef49a1
Compare
pascalwengerter
requested review from
dschmidt and
kulmann
and removed request for
dschmidt
|
Needed to dig a bit deeper here - turns out using the native browser clipboard is only available when using localhost or a secure connection (and hence fails with a self-signed cert/running acceptance tests over HTTP). I searched around and used the workaround mentioned here - using the native API if available, and falling back to the 3rd party implementation that was already used before if it's not available |
|
If the lib doesn't abstract both approaches away for us, is it worth pulling it in at all? It's just a single function invocation anyway, maybe just inline it? |
|
Kudos, SonarCloud Quality Gate passed! |
What do you think of https://vueuse.org/core/useClipboard/ with the |
|
Perfect, obviously 💫 |
pascalwengerter
force-pushed
the
feature/8134
branch
3 times, most recently
from
d367bd9
to
0bf64d8
Compare
| title: $gettext('EOS path copied'), | ||
| desc: $gettext('The EOS path has been copied to your clipboard.') | ||
| }) | ||
| } |
There was a problem hiding this comment.
I know you just copied existing code, but do we really want to expose the EOS string/name to a user?
Should we revisit this, while you're at it?
There was a problem hiding this comment.
Perfect, obviously 💫
I'm not quite sure if I get the isSupported right though - reading https://github.com/vueuse/vueuse/blob/70f55d4e68fd8f100fe844dd883c89b6d6594d03/packages/core/useClipboard/index.ts#L67 it seems like I could omit it since it is basically supported through the legacy option anyways?
Now it's "only" fixing unit tests, no opinion about the EOS translation thought...it's a CERN-internal thing anyways and practically no-one will be able to "casually spin up Reva/oCIS/oc-web on an EOS and then be confused about those strings" ;P
There was a problem hiding this comment.
And from what I remember they literally send each other EOS paths when referencing resources
|
Closed as superseded by #8173, thanks @JammingBen for taking over! |
Description
Figured I'd treat myself to this quickwin before going to bed :D Caniuse approves: https://caniuse.com/?search=navigator.clipboard.writeText
Related Issue
Motivation and Context
I couldn't test the hypothesis of #8134 locally since Safari complained about my dev setup (Chrome/Firefox work fine), please review carefully.
How Has This Been Tested?
Types of changes